Skip to content

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Jun 9, 2025

Turns out that the instrumentation added for gcov can change inlining decisions of the compiler, which results in a mismatch between the profile data CFG and the actual generated CFG between compiles.

There are two functions that suffer from this issue:

  1. _PHP_XXH3_Init: Removing the inline hint fixes this one. In fact, always inlining this makes no sense as there's no real opportunity for specialising. It just bloats the binary and increases I$ pressure. So besides fixing this issue it's beneficial on its own to drop the attribute.
  2. PHP_XXH3_128_Final: Sometimes XXH128_canonicalFromHash gets inlined and sometimes not. Make sure it gets always inlined.

As a side note, since we already have some custom patches to xxhash.h, it would be beneficial to:

  1. Sync upstream
  2. Use a patchfile for PHP-specific additions

Turns out that the instrumentation added for gcov can change inlining
decisions of the compiler, which results in a mismatch between the
profile data CFG and the actual generated CFG between compiles.

There are two functions that suffer from this issue:
1. _PHP_XXH3_Init: Removing the inline hint fixes this one. In fact,
   always inlining this makes no sense as there's no real opportunity
   for specialising. It just bloats the binary and increases I$ pressure.
   So besides fixing this issue it's beneficial on its own to drop the
   attribute.
2. PHP_XXH3_128_Final: Sometimes XXH128_canonicalFromHash gets inlined
   and sometimes not. Make sure it gets always inlined.
@nielsdos nielsdos linked an issue Jun 9, 2025 that may be closed by this pull request
@nielsdos nielsdos requested a review from Girgias June 10, 2025 16:11
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MSTM, but yes would be nice to have an easy way to make sure we don't forget something if we sync up with upstream

@nielsdos
Copy link
Member Author

So we don't forget: #18842

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PGO build fails with xxhash

2 participants